Skip to content

fix: Always place the copy icon on the right side on mobile #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 9, 2025

Conversation

MoritzWeber0
Copy link
Member

@MoritzWeber0 MoritzWeber0 commented Jun 3, 2025

Instead of having the copy icon directly after the text (which produced some issues with multi-line texts), always place the the copy icon on the right side. It's easier to reach for right-handed people.

Not 100% sure if the link icon is still recognized with a connection to the title, so happy to hear any feedback.

Resolves #49

Copy link

netlify bot commented Jun 3, 2025

Deploy Preview for fipguide ready!

Name Link
🔨 Latest commit e26c327
🔍 Latest deploy log https://app.netlify.com/projects/fipguide/deploys/6845abe4f4affd00083d9938
😎 Deploy Preview https://deploy-preview-177--fipguide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@MoritzWeber0 MoritzWeber0 marked this pull request as draft June 3, 2025 19:28
@MoritzWeber0 MoritzWeber0 force-pushed the fix/copy-icon-mobile branch from d71d92e to 0f8b538 Compare June 3, 2025 19:30
@MoritzWeber0 MoritzWeber0 marked this pull request as ready for review June 3, 2025 19:30
@MoritzWeber0 MoritzWeber0 force-pushed the fix/copy-icon-mobile branch from c0ab1ff to 0f8b538 Compare June 3, 2025 19:50
@MoritzWeber0 MoritzWeber0 requested a review from therobrob June 3, 2025 20:00
@therobrob
Copy link
Member

I can understand very well why you changed it. I also find the current placement of the button annoying when it wraps in new line. However, I think the spacing is too large for a short headline or a larger screen width ._.

@therobrob
Copy link
Member

maybe it´s sufficient to have the without wrapping?

@MoritzWeber0
Copy link
Member Author

MoritzWeber0 commented Jun 4, 2025

maybe it´s sufficient to have the without wrapping?

Not trivial. It still looks good for the headings without wrapping, but when wrapping occurs, the first elements takes more space than it needs (even if width: fit-content is set, see w3c/csswg-drafts#191) and the link is pushed to right, making it inconsistent.

image

@MoritzWeber0
Copy link
Member Author

It would be easier to place it left to the text:

image

@MoritzWeber0 MoritzWeber0 force-pushed the fix/copy-icon-mobile branch from 1cb8421 to e26c327 Compare June 8, 2025 15:27
@MoritzWeber0 MoritzWeber0 changed the base branch from main to refactor/define-divider June 8, 2025 15:27
@MoritzWeber0 MoritzWeber0 force-pushed the fix/copy-icon-mobile branch from e26c327 to aa04d6b Compare June 8, 2025 15:28
@MoritzWeber0 MoritzWeber0 force-pushed the refactor/define-divider branch from 1a9e676 to 7152468 Compare June 8, 2025 21:15
@MoritzWeber0 MoritzWeber0 force-pushed the fix/copy-icon-mobile branch from aa04d6b to 715c007 Compare June 8, 2025 21:16
@therobrob therobrob merged commit 0977d3a into refactor/define-divider Jun 9, 2025
1 check passed
@therobrob therobrob deleted the fix/copy-icon-mobile branch June 9, 2025 16:03
@MoritzWeber0 MoritzWeber0 restored the fix/copy-icon-mobile branch June 9, 2025 16:39
therobrob pushed a commit that referenced this pull request Jun 9, 2025
Instead of having the copy icon directly after the text (which produced
some issues with multi-line texts), always place the the copy icon on
the right side. It's easier to reach for right-handed people.

Not 100% sure if the link icon is still recognized with a connection to
the title, so happy to hear any feedback.

Resolves #49

Was originally approved in
#177, but didn't
target the correct branch. Here is another attempt to get it to the main
branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy-Icon Mobile-View
2 participants